Fixed: Unnecessary API Calls and Re-Renders on Patient Page #9311
Conversation
from develop
merge fromdevelop branch
partially revert c9343d5 to fix investigation issues (ohcnetwork#9026)
…ryAPIcallsAndRerenders
WalkthroughThe changes in this pull request focus on optimizing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/Patient/ManagePatients.tsx (1)
294-310: Consider optimizing the object comparison implementation.While the implementation successfully prevents unnecessary updates, it can be improved:
- String conversion of values might lead to false positives (e.g.,
1and"1"would be considered equal).- Sorting entries is unnecessary as
JSON.stringifymaintains key order.Consider this more efficient implementation:
-function compareObjects(obj1: object, obj2: object) { - const entries1 = Object.entries(obj1).map(([key, value]) => [ - key, - String(value), - ]); - const entries2 = Object.entries(obj2).map(([key, value]) => [ - key, - String(value), - ]); - entries1.sort(([key1], [key2]) => key1.localeCompare(key2)); - entries2.sort(([key1], [key2]) => key1.localeCompare(key2)); - return JSON.stringify(entries1) === JSON.stringify(entries2); +function compareObjects(obj1: object, obj2: object) { + return JSON.stringify(obj1) === JSON.stringify(obj2); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Patient/ManagePatients.tsx(2 hunks)
🔇 Additional comments (1)
src/components/Patient/ManagePatients.tsx (1)
312-321: LGTM! The implementation successfully prevents unnecessary API calls.
The integration of paramValue with useQuery effectively addresses the issue of unnecessary API calls by ensuring that the query is only triggered when the parameters actually change.
| const [paramValue, setParamValue] = useState(params); | ||
| const previousValueRef = useRef(params); | ||
|
|
||
| function compareObjects(obj1: object, obj2: object) { |
There was a problem hiding this comment.
If you console 'params' you observe that two params coming in differently but looking the same were being treated as different values. This was causing unnecessary API calls. Upon examining the previous parameter value and the current incoming parameter value, I found they were different. This issue was occurring not only when typing inside the phone number field but also when changing the option, causing unnecessary API calls.
{page: 1, limit: 12, name: undefined, patient_no: undefined, is_active: 'True', …}
{page: '1', limit: 12, name: undefined, patient_no: undefined, is_active: 'True', …}
If you observe the page key value, the previous one is a number, and the current one is a string. This was the problem. I solved it by comparing two objects while ignoring their value types, which now prevents unnecessary API calls
Got the point |
|
@i0am0arunava Can you please share the current status of the PR? Otherwise, the PR will be closed in the next 48 hours, and you will be unassigned from the issue due to inactivity. |
@nihal467 @rithviknishad ,I am on leave due to final sem exam from 3rd to 17th December, and it was approved by Arvind . Please don't close this ,I will resume my work after 17th December |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/Patient/ManagePatients.tsx (1)
292-316: Good solution for preventing unnecessary API calls!The implementation effectively addresses the issue of unnecessary API calls by properly handling type differences in parameter comparisons. However, there are a few optimization opportunities:
- The
compareObjectsfunction could be more efficient by usinguseMemoto cache the sorted entries.- Consider using
useCallbackfor the comparison function to prevent recreation on each render.Here's a suggested optimization:
+ const compareObjects = useCallback((obj1: object, obj2: object) => { + const getSortedEntries = useMemo(() => (obj: object) => { + return Object.entries(obj) + .map(([key, value]) => [key, String(value)]) + .sort(([key1], [key2]) => key1.localeCompare(key2)); + }, []); + + const entries1 = getSortedEntries(obj1); + const entries2 = getSortedEntries(obj2); + return JSON.stringify(entries1) === JSON.stringify(entries2); + }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Patient/ManagePatients.tsx(2 hunks)
🔇 Additional comments (1)
src/components/Patient/ManagePatients.tsx (1)
319-319: LGTM: Proper integration with the optimized state
The modification correctly uses the new paramValue state for API queries, ensuring that the optimization works as intended.
|
👋 Hi, @i0am0arunava, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
Completed in #9400 |
Proposed Changes
prevent Unnecessary API Calls and Re-Renders on Patient Page
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor